Skip to content

(vim): Implement multi-register system and grammar parser#380

Closed
SyedMuzamilM wants to merge 15 commits into
athasdev:masterfrom
SyedMuzamilM:feat/vim-mode
Closed

(vim): Implement multi-register system and grammar parser#380
SyedMuzamilM wants to merge 15 commits into
athasdev:masterfrom
SyedMuzamilM:feat/vim-mode

Conversation

@SyedMuzamilM
Copy link
Copy Markdown
Contributor

@SyedMuzamilM SyedMuzamilM commented Oct 30, 2025

This commit introduces a new multi-register system to store and retrieve content for named registers (a-z, 0-9, ", +, *, _, /). It also initializes default registers (unnamed, yank, delete, black hole, search pattern, system clipboard, selection clipboard). The legacy register is still updated for backward compatibility.

Additionally, this commit implements a grammar-based parser system for Vim commands. This provides a more robust and extensible way to parse Vim commands, and supports future features.

The grammar parser implementation includes:

  • A new Abstract Syntax Tree (AST) for representing commands. - A streaming incremental parser. - A compatibility layer that enables incremental migration between the old and new parser systems. - An executor to execute AST commands.

This commit also introduces new types for working with the grammar parser.

Summary by CodeRabbit

  • New Features

    • Visual Block mode visible in the status indicator
    • Multi-register clipboard with named registers and improved paste options
  • Improvements

    • New grammar-based parser for more reliable recognition of complex Vim commands
    • Enhanced keyboard handling, visual-mode selections, motions, and repeat behavior
    • More consistent yank/delete/paste behavior with better cursor and selection synchronization

This commit introduces a new multi-register system to store and retrieve
content for named registers (a-z, 0-9, ", +, *, _, /). It also
initializes default registers (unnamed, yank, delete, black hole, search
pattern, system clipboard, selection clipboard). The legacy register is
still updated for backward compatibility.

Additionally, this commit implements a grammar-based parser system for
Vim commands. This provides a more robust and extensible way to parse
Vim commands, and supports future features.

The grammar parser implementation includes:

- A new Abstract Syntax Tree (AST) for representing commands. - A
streaming incremental parser. - A compatibility layer that enables
incremental migration between the old and new parser systems. - An
executor to execute AST commands.

This commit also introduces new types for working with the grammar
parser.
mehmetozguldev and others added 8 commits November 1, 2025 21:32
* Migrate components folder into respective feature domains

* Add LLM rules file and create symlinks

* Update documentation folder structure

* Migrate types folder into vertical slice

* Migrate models folder into types folders
This commit introduces a new multi-register system to store and retrieve
content for named registers (a-z, 0-9, ", +, *, _, /). It also
initializes default registers (unnamed, yank, delete, black hole, search
pattern, system clipboard, selection clipboard). The legacy register is
still updated for backward compatibility.

Additionally, this commit implements a grammar-based parser system for
Vim commands. This provides a more robust and extensible way to parse
Vim commands, and supports future features.

The grammar parser implementation includes:

- A new Abstract Syntax Tree (AST) for representing commands. - A
streaming incremental parser. - A compatibility layer that enables
incremental migration between the old and new parser systems. - An
executor to execute AST commands.

This commit also introduces new types for working with the grammar
parser.
@SyedMuzamilM SyedMuzamilM marked this pull request as ready for review November 3, 2025 16:34
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/features/vim/stores/vim-editing.ts (1)

184-203: "P" with characterwise registers now no-ops
pasteAbove returns immediately whenever pasteType !== "line", so a very common flow like yw (characterwise yank) followed by P now does nothing at all. In Vim, P must paste characterwise content before the cursor; with this guard in place the executor calls pasteAbove but it exits before touching the buffer, leaving the editor unchanged. Please branch the logic instead: keep the current linewise path, and add a characterwise path that inserts before the cursor (sharing the cursor/offset bookkeeping with paste).

🧹 Nitpick comments (1)
src/features/vim/hooks/use-vim-keyboard.ts (1)

480-556: Drop verbose console logging from applyMotion.

Lines 480-556: the console.log instrumentation fires on every visual motion and will spam the console in normal usage. Please remove it or wrap it behind an explicit debug flag so production builds stay clean.

-        console.log("=== applyMotion START ===");
-        console.log("Motion keys:", motionKeys);
-
-        const vimStoreBefore = useVimStore.getState();
-        console.log("Before motion - visualSelection:", vimStoreBefore.visualSelection);
-        console.log("Before motion - cursor:", useEditorStateStore.getState().cursorPosition);
-
         let success = false;
 
         if (isNewParserEnabled()) {
           const result = parseGrammar(motionKeys);
           if (result.status === "complete") {
             success = executeAST(result.command);
           }
         } else {
           success = executeVimCommand(motionKeys);
         }
 
         if (!success) {
           console.warn("Failed to execute visual motion:", motionKeys.join(""));
           return false;
         }
 
         const newPosition = useEditorStateStore.getState().cursorPosition;
         const lines = useEditorViewStore.getState().lines;
         const vimStore = useVimStore.getState();
-
-        console.log("After motion - cursor:", newPosition);
-
-
-        // Get fresh visual selection state
         const currentVisualSelection = vimStore.visualSelection;
         const currentVisualMode = vimStore.visualMode;
-
-        console.log("After motion - visualSelection from store:", currentVisualSelection);
 
         if (currentVisualSelection.start) {
           // Line mode: always select full lines
           if (currentVisualMode === "line") {
             const newStart = { line: currentVisualSelection.start.line, column: 0 };
             const newEnd = { line: newPosition.line, column: lines[newPosition.line].length };
-            console.log("Setting line visual selection:", newStart, "to", newEnd);
             vimStore.actions.setVisualSelection(newStart, newEnd);
           } else {
             // Char/block mode: select from start to cursor
             const newEnd = { line: newPosition.line, column: newPosition.column };
-            console.log(
-              "Setting char/block visual selection:",
-              currentVisualSelection.start,
-              "to",
-              newEnd,
-            );
             vimStore.actions.setVisualSelection(currentVisualSelection.start, newEnd);
           }
         }
 
         // Update textarea selection
         const textarea = document.querySelector(".editor-textarea") as HTMLTextAreaElement;
         if (textarea && currentVisualSelection.start) {
           const startOffset = calculateOffsetFromPosition(
             currentVisualSelection.start.line,
             currentVisualSelection.start.column,
             lines,
           );
           const endOffset = newPosition.offset;
-
-          console.log("Textarea offsets - start:", startOffset, "end:", endOffset);
-          console.log(
-            "Setting textarea selection:",
-            Math.min(startOffset, endOffset),
-            "to",
-            Math.max(startOffset, endOffset),
-          );
-
           textarea.selectionStart = Math.min(startOffset, endOffset);
           textarea.selectionEnd = Math.max(startOffset, endOffset);
 
           // Don't dispatch select event - it causes cursor position to be overridden
           // textarea.dispatchEvent(new Event("select"));
         }
-
-        console.log("=== applyMotion END ===\n");
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 39a60e0 and 7deb7dd.

📒 Files selected for processing (15)
  • src/features/vim/components/vim-status-indicator.tsx (2 hunks)
  • src/features/vim/core/core/grammar/ast.ts (1 hunks)
  • src/features/vim/core/core/grammar/compat.ts (1 hunks)
  • src/features/vim/core/core/grammar/executor.ts (1 hunks)
  • src/features/vim/core/core/grammar/motion-kind.ts (1 hunks)
  • src/features/vim/core/core/grammar/normalize.ts (1 hunks)
  • src/features/vim/core/core/grammar/parser.ts (1 hunks)
  • src/features/vim/core/core/grammar/tokens.ts (1 hunks)
  • src/features/vim/core/core/grammar/trie.ts (1 hunks)
  • src/features/vim/core/index.ts (3 hunks)
  • src/features/vim/core/operators/delete-operator.ts (5 hunks)
  • src/features/vim/core/operators/yank-operator.ts (3 hunks)
  • src/features/vim/hooks/use-vim-keyboard.ts (12 hunks)
  • src/features/vim/stores/vim-editing.ts (7 hunks)
  • src/features/vim/stores/vim-store.ts (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/features/vim/core/core/grammar/normalize.ts (1)
src/features/vim/core/core/grammar/ast.ts (1)
  • Command (29-60)
src/features/vim/core/core/grammar/executor.ts (2)
src/features/vim/core/core/grammar/ast.ts (2)
  • Command (29-60)
  • Motion (128-134)
src/features/vim/core/core/grammar/normalize.ts (4)
  • normalize (22-109)
  • effectiveCount (125-144)
  • getRegisterName (152-176)
  • isRepeatable (184-220)
src/features/vim/core/core/grammar/parser.ts (2)
src/features/vim/core/core/grammar/ast.ts (5)
  • RegisterRef (22-24)
  • Target (112-123)
  • ParseResult (211-211)
  • Command (29-60)
  • Motion (128-134)
src/features/vim/core/index.ts (5)
  • actions (161-161)
  • operators (166-166)
  • forcedKinds (163-163)
  • isTextObjectKey (164-164)
  • motions (165-165)
src/features/vim/hooks/use-vim-keyboard.ts (3)
src/features/vim/stores/vim-store.ts (1)
  • useVimStore (310-310)
src/features/vim/core/index.ts (2)
  • isNewParserEnabled (144-144)
  • executeVimCommand (113-113)
src/features/vim/core/core/grammar/executor.ts (1)
  • executeAST (33-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: check format and cargo check

Comment thread src/features/vim/core/core/grammar/compat.ts
Comment thread src/features/vim/core/core/grammar/executor.ts Outdated
Comment thread src/features/vim/core/core/grammar/tokens.ts
Comment thread src/features/vim/hooks/use-vim-keyboard.ts
Comment thread src/features/vim/stores/vim-store.ts Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/features/vim/stores/vim-store.ts (2)

42-42: Consider stronger typing for the AST command.

The any type loses compile-time safety. If the grammar parser defines a Command or ASTNode type, use that instead to catch misuse at build time.

For example:

-  lastRepeatableCommand: any | null; // Store the last Command AST for repeat (.) functionality
+  lastRepeatableCommand: Command | null; // Store the last Command AST for repeat (.) functionality

283-283: Simplify the legacy register sync condition.

The !name check is unnecessary since name is typed as string (not nullable). Simplify to just name === '"'.

-            if (name === '"' || !name) {
+            if (name === '"') {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7deb7dd and c4efc0e.

📒 Files selected for processing (8)
  • src/features/vim/core/core/grammar/compat.ts (1 hunks)
  • src/features/vim/core/core/grammar/executor.ts (1 hunks)
  • src/features/vim/core/core/grammar/tokens.ts (1 hunks)
  • src/features/vim/core/operators/delete-operator.ts (5 hunks)
  • src/features/vim/core/operators/yank-operator.ts (3 hunks)
  • src/features/vim/hooks/use-vim-keyboard.ts (12 hunks)
  • src/features/vim/stores/vim-editing.ts (6 hunks)
  • src/features/vim/stores/vim-store.ts (5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/features/vim/core/operators/yank-operator.ts
  • src/features/vim/core/core/grammar/tokens.ts
🧰 Additional context used
🧬 Code graph analysis (5)
src/features/vim/core/operators/delete-operator.ts (2)
src/features/vim/stores/vim-store.ts (1)
  • useVimStore (316-316)
src/features/editor/extensions/api.ts (1)
  • setCursorPosition (126-154)
src/features/vim/stores/vim-editing.ts (2)
src/features/editor/extensions/api.ts (4)
  • getCursorPosition (122-124)
  • getLines (205-207)
  • setCursorPosition (126-154)
  • getContent (41-43)
src/features/editor/utils/position.ts (1)
  • calculateOffsetFromPosition (37-55)
src/features/vim/core/core/grammar/executor.ts (6)
src/features/vim/core/core/grammar/ast.ts (2)
  • Command (29-60)
  • Motion (128-134)
src/features/vim/core/core/grammar/normalize.ts (4)
  • normalize (22-109)
  • effectiveCount (125-144)
  • getRegisterName (152-176)
  • isRepeatable (184-220)
src/features/vim/stores/vim-editing.ts (1)
  • createVimEditing (34-485)
src/features/editor/utils/position.ts (1)
  • calculateOffsetFromPosition (37-55)
src/features/editor/stores/view-store.ts (1)
  • useEditorViewStore (128-156)
src/features/editor/stores/buffer-store.ts (1)
  • useBufferStore (144-648)
src/features/vim/core/core/grammar/compat.ts (1)
src/features/vim/core/core/grammar/executor.ts (1)
  • executeAST (33-75)
src/features/vim/hooks/use-vim-keyboard.ts (7)
src/features/vim/stores/vim-store.ts (1)
  • useVimStore (316-316)
src/features/vim/core/core/grammar/compat.ts (1)
  • isNewParserEnabled (35-37)
src/features/vim/core/core/grammar/executor.ts (1)
  • executeAST (33-75)
src/features/vim/core/index.ts (1)
  • executeVimCommand (113-113)
src/features/editor/extensions/api.ts (3)
  • getCursorPosition (122-124)
  • getLines (205-207)
  • setCursorPosition (126-154)
src/features/editor/utils/position.ts (1)
  • calculateOffsetFromPosition (37-55)
src/features/editor/stores/view-store.ts (1)
  • useEditorViewStore (128-156)
🔇 Additional comments (6)
src/features/vim/stores/vim-store.ts (6)

9-15: LGTM!

The RegisterContent interface cleanly models Vim register content with appropriate type constraints.


62-75: LGTM!

The multi-register initialization correctly sets up standard Vim registers with sensible defaults.


192-196: LGTM!

The updated signature correctly supports VISUAL BLOCK mode.


290-293: LGTM!

The fallback to an empty register is a safe default for non-existent registers.


301-304: LGTM!

Defaulting to the unnamed register matches Vim's behavior when no register is explicitly selected.


262-288: All callers properly pass the correct source option — verified.

The implementation correctly addresses the past review feedback. All yank operations pass source: "yank" and all delete operations pass source: "delete". This ensures the yank register (0) is only updated for explicit yanks, preventing delete operations from polluting it.

Comment on lines +214 to +227
// Move cursor to last character of pasted content
const newOffset = currentPos.offset + pasteContent.length - 1;
const newLines = newContent.split("\n");
let line = 0;
let offset = 0;

while (offset + newLines[line].length + 1 <= newOffset && line < newLines.length - 1) {
offset += newLines[line].length + 1;
line++;
}

const column = newOffset - offset;
const newPosition = { line, column, offset: newOffset };
setCursorPosition(newPosition);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the P cursor on the first inserted character

For charwise P, Vim leaves the caret on the first character of the inserted text. The new offset calculation adds pasteContent.length - 1, so multi-character inserts now park the cursor on the last character instead. That breaks parity with real Vim and makes follow-up motions land in the wrong place. Please keep the offset at the original cursor position so P behaves correctly.

-        const newOffset = currentPos.offset + pasteContent.length - 1;
+        const newOffset = currentPos.offset;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Move cursor to last character of pasted content
const newOffset = currentPos.offset + pasteContent.length - 1;
const newLines = newContent.split("\n");
let line = 0;
let offset = 0;
while (offset + newLines[line].length + 1 <= newOffset && line < newLines.length - 1) {
offset += newLines[line].length + 1;
line++;
}
const column = newOffset - offset;
const newPosition = { line, column, offset: newOffset };
setCursorPosition(newPosition);
// Move cursor to last character of pasted content
const newOffset = currentPos.offset;
const newLines = newContent.split("\n");
let line = 0;
let offset = 0;
while (offset + newLines[line].length + 1 <= newOffset && line < newLines.length - 1) {
offset += newLines[line].length + 1;
line++;
}
const column = newOffset - offset;
const newPosition = { line, column, offset: newOffset };
setCursorPosition(newPosition);
🤖 Prompt for AI Agents
In src/features/vim/stores/vim-editing.ts around lines 214-227, the code moves
the caret to currentPos.offset + pasteContent.length - 1 which leaves the cursor
on the last pasted character; for charwise P Vim expects the caret on the first
inserted character. Replace the newOffset calculation to use the original
insertion start (e.g., newOffset = currentPos.offset) so the subsequent
line/column computation and setCursorPosition keep the caret on the first
inserted character.

@mehmetozguldev
Copy link
Copy Markdown
Member

Hi @SyedMuzamilM, this PR has been open for a while. Are you still working on this? If so, please rebase on the latest master branch. If not, we can close this and you can reopen when ready. Thanks!

@athasdev athasdev deleted a comment from coderabbitai Bot Dec 24, 2025
@mehmetozguldev mehmetozguldev self-requested a review December 24, 2025 11:23
@SyedMuzamilM SyedMuzamilM deleted the feat/vim-mode branch April 25, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Editor Editor-related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants